Make busybox securityContext configurable#649
Conversation
janhoy
changed the title
|
Alternatively, should perhaps the spec for spec:
cpSolrXmlInitContainer:
image:
registry: public.ecr.aws
repository: my-company/busybox
tag: 1.37.0-custom
imagePullSecret: foo
securityContext:
runAsUser: 1000
runAsGroup: 1000PS: By splitting image into registy, repository and tag, it is easier for downstream users to customize just the registry part. |
There was a problem hiding this comment.
Pull request overview
This PR introduces a configurable securityContext for the cp-solr-xml BusyBox init container so SolrCloud pods can be run with runAsNonRoot: true, while also updating the BusyBox default tag used by the operator/chart.
Changes:
- Add
busyBoxSecurityContextto the SolrCloud API (type + defaults) and propagate it into the generatedcp-solr-xmlinit container. - Update SolrCloud CRD schemas to expose
busyBoxSecurityContext(both config/ and Helm CRD bundles). - Update the documented BusyBox image tag in the Helm chart values.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| helm/solr/values.yaml | Updates the commented BusyBox tag to 1.36.1-glibc. |
| helm/solr-operator/crds/crds.yaml | CRD schema update to include busyBoxSecurityContext. |
| config/crd/bases/solr.apache.org_solrclouds.yaml | Base CRD schema update to include busyBoxSecurityContext. |
| api/v1beta1/common_types.go | Adds ContainerSecurityContext helper type and conversion to corev1.SecurityContext. |
| api/v1beta1/solrcloud_types.go | Adds busyBoxSecurityContext field + defaulting (UID/GID 65534, non-root). |
| api/v1beta1/zz_generated.deepcopy.go | Generated deepcopy support for the new type/field. |
| controllers/util/solr_util.go | Applies the new securityContext to the cp-solr-xml init container. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| Command: []string{"sh", "-c", strings.Join(setupCommands, " && ")}, | ||
| VolumeMounts: volumeMounts, | ||
| Resources: corev1.ResourceRequirements{ | ||
| Requests: volumePrepResources, | ||
| Limits: volumePrepResources, | ||
| }, | ||
| SecurityContext: solrCloud.Spec.BusyBoxSecurityContext.ToSC(), | ||
| } |
There was a problem hiding this comment.
Setting a non-root SecurityContext on the cp-solr-xml init container can break the existing backup-repository prep logic in this same container: when BackupRepositories include volume mounts, setupCommands may include addgroup/adduser/su and chown, which typically require root (or extra capabilities) and will fail when running as UID/GID 65534. Consider either (a) skipping/reworking the user-creation + chown path when BusyBoxSecurityContext enforces non-root (e.g., rely on FSGroup and only do a write-test with a clear error), or (b) splitting the permission-fixup into a separate, optionally-root init container so cp-solr-xml can remain non-root for runAsNonRoot: true pods.
| if spec.BusyBoxSecurityContext == nil { | ||
| c := ContainerSecurityContext{} | ||
| spec.BusyBoxSecurityContext = &c | ||
| } | ||
| changed = spec.BusyBoxSecurityContext.withDefaults(DefaultBusyBoxUserId, DefaultBusyBoxGroupId, DefaultBusyBoxRunAsNonRoot) || changed |
There was a problem hiding this comment.
New defaulting for BusyBoxSecurityContext introduces user-visible behavior (init container securityContext defaults to UID/GID 65534 and runAsNonRoot: true), but there doesn’t appear to be controller/unit test coverage asserting the generated StatefulSet’s cp-solr-xml init container gets this securityContext (and that overrides work). Adding assertions in the existing controller tests that inspect init containers would help prevent regressions, especially given the interaction with PodSecurityContext/FSGroup and backup-repo prep.
Draft PR, only code, not docs, no helm support.
The
busyboxofficial image runs as root incp-solr-xmlinit-container, and there is no way to configure it otherwise, other than point to a different image that has been manipulated as non-root.By adding a
SecurityContextfor the init container defaulting to thenobodyuser and settingrunAsNonRoot: true, we have a good default. By also making thesecurityContextconfigurable, we allow for people to switch to a different image with other UID etc. Example:Fixes #582